-
Notifications
You must be signed in to change notification settings - Fork 823
boot: zephyr: config: Allow custom MCUBOOT_BOOT_MAX_ALIGN #2453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
#if defined(CONFIG_MCUBOOT_BOOT_MAX_ALIGN) && CONFIG_MCUBOOT_BOOT_MAX_ALIGN > 0 | ||
#define MCUBOOT_BOOT_MAX_ALIGN CONFIG_MCUBOOT_BOOT_MAX_ALIGN | ||
#elif DT_HAS_CHOSEN(zephyr_flash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have these dt_ macros to use with Kconfig, can we maybe move the entire logic to Kconfig?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, yes but this fix should make that backwards compatible, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. So you set the Kconfig to defaults as the logic states here, but people can still override it. Wouldn't that make it backwards compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way we also have only one place where it is set: the Kconfig; there would no "if Kconfig is 0 then", because it could only be 0 if we allow somebody to set it as such in Kconfig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I missing something here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR per your suggestion. A single Kconfig symbol, which default to the chosen flash's write-block-size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a side-note, we're backwards compatible like this indeed, but not forwards compatible (which was te case before my change).
So if this change is pulled in without zephyrproject-rtos/zephyr#96001 the behavior changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, a specific commit of MCUboot is valid with a specific commit of zephyr, as the west.yml file points to
6a6b7a6
to
610b1db
Compare
610b1db
to
bec3cdf
Compare
Allow setting CONFIG_MCUBOOT_BOOT_MAX_ALIGN to apply a programmable block in flash. Move the chosen flash write block size logic to Kconfig. Signed-off-by: Pieter De Gendt <[email protected]>
bec3cdf
to
e1ee957
Compare
@pdgendt We will need release notes entry for this change. |
Allow setting CONFIG_MCUBOOT_BOOT_MAX_ALIGN to apply a custom programmable block in flash.